-
Notifications
You must be signed in to change notification settings - Fork 769
[SYCL][DOC] Refactor OCL global variable spec #5659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We decided that the OpenCL API Specification should not contain detailed information about how the APIs interact with SPIR-V. Instead, we want this information in the OpenCL SPIR-V Environment Specification. Change this extension specification accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks good to me. 👍
At some point we may decide that the modifications to the SPIR-V environment spec belong more with whatever extension or tooling describes support SPV_INTEL_global_variable_decorations rather than here, but we can work that out later if necessary. The important thing for now is that the name resolution will be documented in the SPIR-V environment spec.
parameter which identifies the variable. This parameter is interpreted as | ||
follows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider saying explicitly that this section only applies for programs created via SPIR-V, perhaps by adding to the last sentence:
For a
cl_program
created from SPIR-V, this parameter is interpreted as follows...
Debatably this is unnecessary because this section is in the SPIR-V environment specification, but I still think it is clearer to say it explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Done in 39cad4d
Comment from Ben, clarify that these rules apply only if the module was created from SPIR-V.
Yes, I agree. It might be better to create a GitHub "team" for the OpenCL spec reviewers, similar to the way we have "intel/dpcpp-spirv-reviewers" for SPIR-V specs. We could then add @bashbaug to that team and potentially others. |
We decided that the OpenCL API Specification should not contain
detailed information about how the APIs interact with SPIR-V. Instead,
we want this information in the OpenCL SPIR-V Environment
Specification.
Change this extension specification accordingly.